-
Notifications
You must be signed in to change notification settings - Fork 558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] ES Decorators and React #31
Conversation
Other ways of providing extra data are HOCs and Render Props There are old-ES-Decorators-style HOCs (
And also Relay HOCs ( |
// After: | ||
@React.pure | ||
@React.async | ||
export default class MyComponent extends React.Component {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be done by
export default React.pure(React.async(MyComponent));
Class decorator is a function that consumes a class and may return a class. If there is a HOC that consumes a React component and returns a stateful react component, then you can already use it as a class decorator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decorators:
@React.pure
@React.async
export default class MyComponent extends React.Component {
// ...Maybe hundreds of lines...
}
Nested Calls:
class MyComponent extends React.Component {
// ...Maybe hundreds of lines...
}
export default React.pure(React.async(MyComponent));
Nested Calls are harder to write and read, produce worse diffs and requires you to scroll through possibly hundreds of lines to be seen.
Whatever can be done with decorators can already be done in one way or another. I just believe that decorators are easier to use. Consider it sugar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@streamich everything can be done by functions calls at the end of the day
const NavigationStyled = withStyles(styles)(NavigationComponent);
const NavigationStyledConnected = connect(
mapStateToProps,
mapDispatchToProps
)(NavigationStyled);
const NavigationStyledConnectedWithRouter = withRouter(NavigationStyledConnected)
export { NavigationStyledConnectedWithRouter as Navigation }
vs
@withStyles(styles)
@connect(mapStateToProps, mapDispatchToProps)
@withRouter()
class Navigation extends React.Component { }
export Navigation
I would prefer to focus on how this would improve our code base and workflows. Everything could be a function call at the end of the day, but fundamentality having something like this would improve the coding.
P.S: despite the current decorator state and opinions about mutating things inside or not, decorators are not evil, engineers are evil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be interested in pipeline operator.
P.S. I like decorators and use them all the time, I just don't really understand what is the discussion about? If you like @withRouter
as a decorator, you can already use it. Or is about bringing decorators into React core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are using Stage 0 Decorators. TypeScript doesn't have Stage 2 Decorators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use whatever decorators TypeScript supports, but thanks for the info, will look into the differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@streamich as an Elixir developer I definitely prefer the pipe operator but that is not my point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pipe operators are nice, but still, I would prefer to use decorators at class level. I think that pipe operators are more useful for minor operations such as helper function calls, but they would look horrible at class level.
```javascript | ||
class MyComponent extends Component { | ||
@React.ref | ||
myTextInput; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to just assign the value?
myTextInput = React.ref();
Also, I believe that instance property decorators are broken in TypeScript, correct me if I'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the other comment for explanation.
TypeScript has support for old ES Decorators and it's under a flag. They jumped the gun and implemented a non-standard and fed it to people through Angular. TypeScript's mess doesn't and shouldn't affect ECMA262 or React ecosystems.
|
||
render() { | ||
return ( | ||
<TextInput ref={this.refs.myTextInput} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need .refs
here?
<TextInput ref={this.myTextInput} />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
myTextInput
is typed ?TextInput
. this.refs
contains hints for every property decorated with @React.ref
. React uses those hints to automatically build a ref callback like (ref) => { instance[propertyName] = ref; }
.
Naming could be different or this could be a stupid idea after all.
|
||
@withRouter | ||
@connect(/* ... */) | ||
export default class MyComponent extends Component { /* ... */ } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong, but you can already do this, if withRouter()
and connect()
return stateful class components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also correct me if I'm wrong, but) ES Decorators spec has changed and their current implementations are no longer valid decorators. Current decorators do not return a new class. (AFAIK, finalising the decoration with a new class is still possible though.)
I'd like to note that the main aim of the RFC is talk about not React's code but its ecosystem. I can imagine many conclusions for this RFC:
While I follow many well-considered React people, I haven't seen anyone talking about decorators and how they can (or maybe can't) benefit React. I just wanted to start a discussion and get people thinking with decorators. I will edit the RFC as we further the discussion. I believe this will be an easier discussion after Stage 2 Decorators support for Babel 7 is complete: babel/proposals#11 Until then it will not be possible to provide POCs. |
As you note, Decorators are stage 2. I believe the main reason they haven't been discussed more broadly is because they're still viewed as unstable and likely to cause churn. The confusion between stage 0 and stage 2 decorators in the comments above seems to be evidence to that point, it seems very possible that they'll undergo further changes if and when they progress to stage 3. And if it's not currently possible to provide POCs because they're not supported anywhere, then that again seems like evidence that this conversation should be happening sometime in the future. |
That's the important part. We can either wait until TC39 finalises the spec, or shape the spec by thinking on valid use cases. It would be great if we could discuss how we could benefit from decorators and provide TC39 with constructive inputs. |
@vcarl the reason why I would love React team to talk about this so the specification moves faster and we can benefit from this feature, but because we keep looking for workaround (or rather other way to do it) we do not get there 😞 From my code example, either pipe operator or decorators but the current way to do is so ugly and easy to error prune to be honest. |
HOCs with DecoratorsHOC (Note: This is NOT a valid Stage 2 Decorator) const injectFoo = (value) => (Base) =>
class Wrapper extends Component {
render() {
return <Base foo={value} />;
}
};
const injectBar = /* ... */;
const injectBaz = /* ... */;
class MyComponent extends Component {
render() {
const { foo, bar, baz } = this.props;
}
}
export default injectFoo('foo')(injectBar('bar')(injectBaz('baz')(MyComponent))); HOC w/ Stage 2 Decorators const injectFoo = (value) => (classDescriptor) => ({
...classDescriptor,
// Finisher is a HOC
finisher: (Base) =>
class Wrapper extends Component {
render() {
return <Base foo={value} />;
}
},
});
const injectBar = /* ... */;
const injectBaz = /* ... */;
@injectFoo('foo')
@injectBar('bar')
@injectBaz('baz')
export default class MyComponent extends Component {
render() {
const { foo, bar, baz } = this.props;
}
} Preventing Prop Name Collision with Property DecoratorsApollo has a HOC named "graphql" and it injects a prop named "data". To prevent collisions you have to pass an options object for each use: import { graphql } from 'react-apollo';
class MyComponent extends Component {
render() {
const { foo, bar, baz } = this.props;
}
}
export default compose(
graphql(foo, { name: 'foo' }),
graphql(bar, { name: 'bar' }),
graphql(baz, { name: 'baz' }),
)(MyComponent); This could be simplified with property decorators: import { graphql } from 'react-apollo';
const graphqlProp = (query, options) => (elementDescriptor) => {
const propName = elementDescriptor.key;
return {
...elementDescriptor,
propertyDescriptor: {
...elementDescriptor.propertyDescriptor,
writable: false,
get() {
return this.props[propName];
},
},
// Finisher is a wrapped HOC
finisher: (Base) => graphql(query, {
...options,
name: propName,
})(Base),
}
};
export default class MyComponent extends Component {
@graphqlProp(foo) foo;
@graphqlProp(bar) bar;
@graphqlProp(baz) baz;
render() {
// Both work
const { foo, bar, baz } = this.props;
const { foo, bar, baz } = this;
}
} |
// Before: | ||
export default class MyComponent extends React.PureComponent { | ||
return ( | ||
<AsyncMode> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that AsyncMode applies to the whole subtree. It's not specific to a component. You'd normally only use it once at the top. So I think it's not very relevant to this proposal.
I use decorators a lot for providing access to some context, such as confirm dialogues and status messages. A nice bonus is that they can render differently in different contexts. Also, I prefer it to using render props. With render props I don't have access to the received state from @withContext('foo', MyContext)
class Foo extends Component {
render() {
return (
<div>
{this.props.foo.bar}
</div>
)
}
} This would be just as cool however: class Foo extends Component {
static withContext = {
foo: MyContext
}
render() {
return (
<div>
{this.props.foo.bar}
</div>
)
}
} |
A WIP RFC to discuss how ES Decorators can benefit React ecosystem.